Skip to content

treewide: Remove use of apple_sdk framework stub paths#399040

Merged
emilazy merged 10 commits into
NixOS:stagingfrom
booxter:remove-injection-of-old-darwin-framework-stubs
Apr 19, 2025
Merged

treewide: Remove use of apple_sdk framework stub paths#399040
emilazy merged 10 commits into
NixOS:stagingfrom
booxter:remove-injection-of-old-darwin-framework-stubs

Conversation

@booxter

@booxter booxter commented Apr 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

This patch covers all packages that use stub paths in some way in their
build instructions.

(Except those listed in this comment.)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions Bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Apr 16, 2025
@nix-owners nix-owners Bot requested review from Profpatsch and natsukium April 16, 2025 02:03
@github-actions github-actions Bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels Apr 16, 2025
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from cb54e4f to 5803db0 Compare April 16, 2025 02:10
@booxter booxter changed the base branch from master to staging April 16, 2025 02:10
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 5803db0 to 28e3371 Compare April 16, 2025 04:25
@booxter

booxter commented Apr 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I've manually checked these packages on aarch64-darwin. Results are as follows:

vengi-tools: broken since 24.05: https://hydra.nixos.org/build/260818375
cardpeek: broken since 24.11: https://hydra.nixos.org/build/278953995
gildas: builds
nextpnr: broken in staging-next: https://hydra.nixos.org/build/294373417
odin: builds
opensc: builds
bullet: builds
bullet-roboschool: broken in 24.05: https://hydra.nixos.org/build/260816205
libsamplerate: builds
netcdffortran: builds
jaxlib: not utilized since the switch to -bin
pyscard: builds
bazel-test-cpp: passes
xorg.xorgserver: broken in staging-next: https://hydra.nixos.org/build/294644953 (dependency issue)

I've also checked for those already broken packages that failures are similar to what can be seen on hydra. So I hope this doesn't break anything that wasn't broken before.

OpenSC can be cleaned up because it detects the right path for PSCS if configure option is not passed:

opensc> PC/SC default provider:  /System/Library/Frameworks/PCSC.framework/PCSC
opensc> PKCS11 default provider: /nix/store/2mph7dm80lksszb10gf4mank6yp73zfp-opensc-0.26.1/lib/opensc-pkcs11.so
opensc> PKCS11 onepin provider:  /nix/store/2mph7dm80lksszb10gf4mank6yp73zfp-opensc-0.26.1/lib/onepin-opensc-pkcs11.so

bullet can be cleaned up more, need to remove patchPhase sections that tried to work around old SDK.

@booxter

booxter commented Apr 16, 2025

Copy link
Copy Markdown
Contributor Author

Not sure if this commit / PR should have treewide as its title tag. Please advise.

@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 28e3371 to 22bdec6 Compare April 16, 2025 13:04
@booxter booxter changed the title darwin: Remove use of apple_sdk framework stub paths treewide: Remove use of apple_sdk framework stub paths Apr 16, 2025
@NickCao

NickCao commented Apr 16, 2025

Copy link
Copy Markdown
Member

The PR title should be treewide, but the changes should be in individual commits for individual packages.

@booxter booxter marked this pull request as draft April 17, 2025 21:25
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch 4 times, most recently from 712c5a4 to 05b7ccf Compare April 18, 2025 01:51
@booxter booxter marked this pull request as ready for review April 18, 2025 01:51
@booxter

booxter commented Apr 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I separated each package into a separate commit. I also split out more elaborate pieces out into individual PRs:

With these merged, we should have no packages using old apple_sdk stub paths in-tree.

@nix-owners nix-owners Bot requested a review from Astavie April 18, 2025 01:59
@Astavie

Astavie commented Apr 18, 2025

Copy link
Copy Markdown
Contributor

How come this is being removed? I remember the odin package requiring these inputs to compile on macos, has there been a recent change so that is not required anymore?

@yzx9

yzx9 commented Apr 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

How come this is being removed? I remember the odin package requiring these inputs to compile on macos, has there been a recent change so that is not required anymore?

Yes, the Darwin SDK has been fully reworked in #346043. The old-style SDKs were just no-op placeholders now.

emilazy
emilazy reviewed Apr 18, 2025
View reviewed changes

@emilazy emilazy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great on the whole, thanks! Some nits.

Comment thread pkgs/by-name/od/odin/package.nix Outdated
Comment thread pkgs/by-name/op/opensc/package.nix Outdated
Comment thread pkgs/development/libraries/bullet/default.nix Outdated
Comment thread pkgs/development/libraries/bullet/roboschool-fork.nix Outdated
@smaret

smaret commented Apr 18, 2025

Copy link
Copy Markdown
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 399040 --package gildas

aarch64-darwin

✅ 1 package built:
  • gildas

smaret
smaret approved these changes Apr 18, 2025
View reviewed changes

@smaret smaret left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change in gi/gildas/package.nix looks good to me. Thanks!

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Apr 18, 2025
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 05b7ccf to 4fe11b6 Compare April 18, 2025 23:42
@booxter booxter requested a review from emilazy April 18, 2025 23:43
emilazy
emilazy approved these changes Apr 19, 2025
View reviewed changes

@emilazy emilazy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! I plan to do a nixpkgs-review and then merge within a day or two. Some nits if you feel like addressing them before then, but I won’t block on them.

Comment thread pkgs/by-name/od/odin/package.nix Outdated
Comment thread pkgs/development/python-modules/pyscard/default.nix Outdated
Comment thread pkgs/development/libraries/bullet/default.nix Outdated
Comment thread pkgs/development/libraries/bullet/roboschool-fork.nix Outdated
booxter added 4 commits April 18, 2025 21:01
@booxter
python3Packages.pyscard: clean up apple_sdk stubs
d4ef6cf 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
netcdffortran: clean up apple_sdk stubs
9f0c288 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
libsamplerate: clean up apple_sdk stubs
8c64351 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
xorg.xorgserver: clean up apple_sdk stubs
cc1d31d 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 4fe11b6 to 10a73ad Compare April 19, 2025 01:02
booxter added 6 commits April 18, 2025 21:11
@booxter
odin: clean up apple_sdk stubs
84b5f48 
Also, patched out all impure paths in the linker.cpp file for darwin.

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
bullet{,roboschool}: clean up apple_sdk stubs
a5cd73e 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
gildas: clean up apple_sdk stubs
a8fc944 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
opensc: clean up apple_sdk stubs
7ad1b28 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
bazel-test-cpp: clean up apple_sdk stubs
2ea4434 
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter
python3Packages.jaxlib: clean up apple_sdk stubs
d1ee8a7 
Note: jaxlib currently points to jaxlib-bin so this code is not utilized
for regular jaxlib installations.

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 10a73ad to d1ee8a7 Compare April 19, 2025 01:11
emilazy
emilazy approved these changes Apr 19, 2025
View reviewed changes

@emilazy emilazy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, when I said I’d run nixpkgs-review I hadn’t looked at the rebuilds total. But I built every directly touched package on x86_64-darwin except for xorg.xorgserver (didn’t want to build on staging and too lazy to test it on top of the libAppleWM change, plus it was broken before anyway), bullet-roboschool (broken on x86_64-darwin), and gildas (just takes too long). Thank you!

@emilazy emilazy merged commit bd79aca into NixOS:staging Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilazy emilazy emilazy approved these changes
@Profpatsch Profpatsch Awaiting requested review from Profpatsch
@natsukium natsukium Awaiting requested review from natsukium
@lovek323 lovek323 Awaiting requested review from lovek323
@layus layus Awaiting requested review from layus
@bzizou bzizou Awaiting requested review from bzizou
@prusnak prusnak Awaiting requested review from prusnak
@bjornfor bjornfor Awaiting requested review from bjornfor
@michaeladler michaeladler Awaiting requested review from michaeladler
@aforemny aforemny Awaiting requested review from aforemny
@Astavie Astavie Awaiting requested review from Astavie
+1 more reviewer
@smaret smaret smaret approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@booxter @NickCao @Astavie @yzx9 @smaret @emilazy @wegank